Skip to content

Remove syntax classification lit-based tests #1966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

StevenWong12
Copy link
Contributor

The follow-up PR for #1953.

Also remove the lit-test-helper in codebase. 🥳

@StevenWong12 StevenWong12 requested a review from ahoppen as a code owner July 31, 2023 05:06
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just looked through the test cases that you didn’t translate to XCTest and I agree that most of them are redundant or pointless with the new SwiftParser implementation. The only exception is that I’d like to have a test case where an escaped keyword is used as an identifier.


func keywordInCaseAndLocalArgLabel(_ for: Int, for in: Int, class _: Int) {
// CHECK: <kw>func</kw> <id>keywordInCaseAndLocalArgLabel</id>(<kw>_</kw> <id>for</id>: <type>Int</type>, <id>for</id> <id>in</id>: <type>Int</type>, <id>class</id> <kw>_</kw>: <type>Int</type>) {
switch(`for`, `in`) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some test case with escaped identifiers would be good to keep.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's good👍. I added two in my new commit in #1953.

@ahoppen
Copy link
Member

ahoppen commented Jul 31, 2023

Otherwise: Thanks a lot for getting rid of lit-test-helper 🚀

@ahoppen
Copy link
Member

ahoppen commented Aug 3, 2023

@StevenWong12 Could you rebase this PR?

Also remove `lit-test-helper` in the codebase
@StevenWong12 StevenWong12 force-pushed the remove_syntax_classification_lit_test branch from 26e1416 to da586f2 Compare August 4, 2023 00:50
@ahoppen
Copy link
Member

ahoppen commented Aug 4, 2023

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge August 4, 2023 01:22
@ahoppen
Copy link
Member

ahoppen commented Aug 4, 2023

@swift-ci Please test Windows

@ahoppen
Copy link
Member

ahoppen commented Aug 4, 2023

@swift-ci Please test macOS

@ahoppen ahoppen merged commit bf1f861 into swiftlang:main Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants